Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify permission prefix usage in function documentation for OxPlayer.hasPermission #235

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Rhydium
Copy link

@Rhydium Rhydium commented Jan 14, 2025

Updated the documentation to explicitly state that the group.<groupname> prefix applies to the permission input for the OxPlayer.hasPermission function. This change ensures clearer understanding of how permissions are processed and avoids potential confusion for developers.

Updated the documentation to explicitly state that the `group.<groupname>`
prefix applies to the permission input for the function, not the function itself.
This change ensures clearer understanding of how permissions are processed
and avoids potential confusion for developers.
@Leah-UK
Copy link
Member

Leah-UK commented Jan 15, 2025

Seems good to me. This along with some other areas need to be fleshed out.

@thelindat
Copy link
Member

It would make more sense for SetGroupPermission to mention what the permission string is called (i.e. adding the handcuff permission to the police group results in group.police.handcuff).

@Rhydium
Copy link
Author

Rhydium commented Jan 15, 2025

It would make more sense for SetGroupPermission to mention what the permission string is called (i.e. adding the handcuff permission to the police group results in group.police.handcuff).

I personally think some kind of reminder here makes sense as well, because developers might not be looking at SetGroupPermission when they are trying to use hasPermission. Maybe a better format would be to remove the special call-out and add an example codeblock to both functions?

@thelindat
Copy link
Member

An example would be fine but the callout doesn't really make sense; hasPermission isn't setup for groups, it's for permissions which are currently only set by groups. The note about input requiring group.groupName isn't really accurate since I can very easily add other permissions that wouldn't need that.

@Rhydium
Copy link
Author

Rhydium commented Jan 16, 2025

I’ve replaced the callout with example code. Could you please review it again? I believe adding example code throughout the documentation would be beneficial, as it helps lower the barrier to entry for the entire framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants